Skip to content

fix(publicip): retry fetching information on failure - #3388

Open
yorah wants to merge 1 commit into
passteque:masterfrom
yorah:fix-publicip-retry
Open

fix(publicip): retry fetching information on failure#3388
yorah wants to merge 1 commit into
passteque:masterfrom
yorah:fix-publicip-retry

Conversation

@yorah

@yorah yorah commented Jul 9, 2026

Copy link
Copy Markdown

Description

The public IP information is fetched only once per tunnel-up (onTunnelUpRunOnce). That single fetch runs seconds after reconnection, when the tunnel is often still flaky, and if it fails, the data stays empty until the next tunnel-up, potentially days later, with /v1/publicip/ip serving {"public_ip":""} the whole time (breaking Homepage widgets and monitoring, as reported in #3300).

Real-world example (v3.41.1, WireGuard/AirVPN): after a ~25 min provider outage, the healthcheck-triggered reconnect succeeded, but the tunnel-up fetch timed out on all four sources (all fetchers failed: … Client.Timeout exceeded). The tunnel stabilized seconds later, yet the IP stayed empty for days because nothing ever retried.

This PR makes the fetch self-healing:

  • on failure, a retry is scheduled with exponential backoff: 10s, doubling, capped at 5 minutes, until success;
  • a new tunnel-up trigger supersedes any pending retry and resets the backoff;
  • retries stop when the tunnel goes down (ClearData) or if the public IP feature is disabled.

No new settings (no wiki change needed), and zero extra API load when fetches succeed. Includes two unit tests (pass with -race); golangci-lint clean; docker build . succeeds.

Issue

Fixes #3300 (also previously reported as #1855, #2848).

Assertions

  • I am aware that any changes to settings should be reflected in the wiki

@qdm12

qdm12 commented Jul 11, 2026

Copy link
Copy Markdown
Member

Thanks for the contribution. Unfortunately that's a lot of added complexity for a bandaid fix which does not address the true issue here.

That single fetch runs seconds after reconnection, when the tunnel is often still flaky

It should not be "flaky". The "full" healthcheck (TLS+DNS check) passed before fetching the public ip information, so the tunnel should be already working well. If there was an outage or unhealthy connections beforehand, it shouldn't matter for this new connection.

On top of this, in issue #3300 you can see there are other issues such as cannot get version information so focusing exclusively on the public ip part is not good enough. We should address the root cause, mostly related to the http client timeout.

If this is still unresolved with the latest image which should have a 15s timeout (double check with log timestamps), we can proceed with the bandaid fix for the public ip but with a fixed number of retries (3) and no retry wait time, to keep code simple - if it still fails after 3 x 15s retries, it probably won't work ever.

@yorah

yorah commented Jul 11, 2026

Copy link
Copy Markdown
Author

Thanks for your feedback!

To be more precise, the few times I observed this behavior was when AirVPN had an outage. And I think the "flakiness" was on their side (due to everyone trying to reconnect to the serve rthat just went up at the same time).

I will re-check my logs when I get back home, and report back. I remember I am using 3.41.1, which I believe is the last release.

@qdm12

qdm12 commented Jul 11, 2026

Copy link
Copy Markdown
Member

try the latest image It's about 7 months ahead of v3.41.1 and not the same as v3 or v3.41.1

@yorah

yorah commented Jul 21, 2026

Copy link
Copy Markdown
Author

So after one week of running the latest image (I was previously pinned to 3.41.1), I haven't had the issue anymore. I'll close this PR as unneeded, and comment back if I ever encounter the problem again. Thanks!

@yorah yorah closed this Jul 21, 2026
@yorah yorah reopened this Jul 26, 2026
@yorah

yorah commented Jul 26, 2026

Copy link
Copy Markdown
Author

Reopening: I got this again on the latest image (commit 78f6076, built 2026-07-19).

This was the AirVPN outage scenario I mentioned earlier. It started here, with the small ICMP check failing, followed by path MTU discovery getting nothing back and a TCP dial to a bare IP timing out:

01:45:05 WARN [vpn] restarting VPN because it failed to pass the healthcheck: small periodic check: all check tries failed:
01:45:11 INFO [MTU discovery] reverting VPN interface tun0 MTU to 1320 (due to: PMTUD failed with both ICMP and TCP)
01:45:17 WARN [vpn] restarting VPN because it failed to pass the healthcheck: startup check: all check tries failed: parallel attempt 1/2 failed: dialing: dial tcp4: lookup github.com: i/o timeout, parallel attempt 2/2 failed: dialing:
dial tcp4 104.16.133.229:443: i/o timeout

Nothing was reachable through the tunnel for about ten minutes, by name or by IP, and the VPN restarted 30 times in that window. Then the connection came back:

01:56:30 WARN [vpn] restarting VPN because it failed to pass the healthcheck: ...
01:56:48 ERROR [vpn] getting public IP address information: fetching information: all fetchers failed:
ipinfo: Get "https://ipinfo.io/": context deadline exceeded
ifconfig.co: Get "https://ifconfig.co/json": context deadline exceeded
ip2location: Get "https://api.ip2location.io/": dial tcp [2606:4700:20::681a:681]:443: connect: network is unreachable
cloudflare: Get "https://speed.cloudflare.com/meta": context deadline exceeded

The tunnel passed its startup healthcheck at 01:56:30, and the public IP fetch still failed about 15 seconds later on all four APIs. The 15s timeout is clearly applied and does not help, because the connection was not slow, it was
still not carrying traffic reliably in those first seconds after reconnect. So a passing healthcheck does not mean the fetch will succeed.

My guess is that this is a thundering herd effect: when an AirVPN server comes back, every client that was on it reconnects at the same time, and the exit stays congested for a short while afterwards. That would explain why three of
the four APIs hit the deadline rather than failing outright, and why it clears up on its own shortly after.

Worth noting that the periodic healthcheck already does exactly what this PR proposes: withRetries with escalating 10s, 15s, 30s timeouts, with the comment "20s timeout in case the connection is under stress". The public IP fetch is a
strictly heavier operation, a full HTTPS GET with a response body against third party hosts rather than a TLS handshake against an anycast edge, and it gets a single attempt. In my log the healthcheck handshaked with cloudflare.com:443
successfully while the GET to speed.cloudflare.com/meta timed out in the same window. Same network, same seconds, just a heavier request.

What do you think about that analysis? I can do other tests if you have other ideas.

Also happy to rewrite this PR as you suggested, with 3 fixed retries and no wait time (althouhg in a thundering herd problem, adding some wait could be beneficial). Let me know and I will force-push.

@yorah

yorah commented Jul 26, 2026

Copy link
Copy Markdown
Author

On addressing the root cause, here is a proposal that also covers the version check. Tell me if you like it before I write it.

Both failing (cannot get version information from #3000, and mine) fetches sit in onTunnelUp, back to back under vpnCtx, both idempotent GETs with a 15s deadline and no retry. The version one is worse, since l.versionInfo = false means one failure loses it until restart.

Proposal: promote your withRetries from internal/healthcheck/checker.go into a small shared package, generic over the result:

type Policy struct {
    Timeouts []time.Duration // per-attempt deadline; len = max attempts
    Waits    []time.Duration // optional pause before attempt i; nil for none
}

func Do[T any](ctx context.Context, policy Policy, logger Logger, name string,
    op func(ctx context.Context, attempt int) (T, error)) (T, error)

Three users: healthcheck unchanged with {10s, 15s, 30s} and no waits, publicip wrapping FetchInfo above ResilientFetcher, and version wrapping GetMessage.

I looked at putting a retrying RoundTripper on the shared httpClient instead, but all attempts would then share the single 15s request context, so every call site deadline would need widening anyway, and the port forwarding calls are
not all idempotent.

This also lets me drop the retryWanted atomic and the ClearData change. vpnCtx is already cancelled on tunnel down, so retries stop by themselves. That was the unnecessary complexity you flagged.

If the shape works I will rewrite the PR this way. If you would rather keep it to publicip with 3 fixed retries and no shared package, say so and I will do that instead. Or if you have other ideas, I'm also interested :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: getting public IP address information: fetching information: all fetchers failed: ... context deadline exceeded

2 participants